feat: sensitive secret configs by yanksyoon · Pull Request #219 · canonical/github-runner-image-builder-operator

yanksyoon · 2026-04-17T04:59:14Z

Applicable spec:

Overview

Use Juju secrets for sensitive configuration values

Rationale

Juju secret is a secure way to manage sensitive values

Juju Events Changes

Module Changes

Library Changes

Checklist

The charm style guide was applied
The contributing guide was applied
The changes are compliant with ISD054 - Managing Charm Complexity
The documentation for charmhub is updated.
The PR is tagged with appropriate label (urgent, trivial, senior-review-required, documentation)
The docs/changelog.md is updated with user-relevant changes in the format of keep a changelog v1.1.0
The application version is incremented in app/pyproject.toml

No app changes

Copilot

Pull request overview

This PR migrates sensitive OpenStack password configuration from plaintext charm config to Juju secrets, updating runtime parsing and test scaffolding accordingly.

Changes:

Switch openstack-password config to type: secret and update documentation in charmcraft.yaml.
Update OpenStack clouds config parsing to read the password from a Juju secret via model.get_secret(...).get_content().
Update unit/integration test fixtures to create and grant the OpenStack password secret.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/state.py`	Fetch OpenStack password from a Juju secret and raise config errors on secret lookup/content failures.
`charmcraft.yaml`	Change `openstack-password` config option to `type: secret` and document required secret content format.
`docs/changelog.md`	Add changelog entry noting Juju secrets usage for sensitive config.
`tests/unit/conftest.py`	Update harness fixture to create/grant a user secret and set config to the secret id.
`tests/unit/factories.py`	Update mock charm config to use a secret id and mock `model.get_secret()` content.
`tests/unit/test_state.py`	Add unit tests for secret lookup/access errors and missing secret content key.
`tests/integration/conftest.py`	Add integration fixtures for OpenStack password secret, pass secret id via config, and grant secret to the app.

Comments suppressed due to low confidence (1)

tests/integration/conftest.py:436

The declared return type says the third element is a set, but charmhub_info[...].keys() returns a dict_keys view. Either convert it (e.g., set(...)) or adjust the return annotation to match what’s actually returned, otherwise mypy/type-checking can fail.

async def _prepare_charmhub_app_config(ops_test, app_config: dict) -> tuple[str, dict, set]:
    """Prepare the application config for charmhub deployment.

    Args:
        ops_test: The pytest operator test instance.
        app_config: The base application configuration.

    Returns:
        A tuple of (channel, prepared_config, config_options).

    """
    charmhub_channel = "edge"
    ret_code, stdout, stderr = await ops_test.juju(
        "info", "--format", "json", "--channel", charmhub_channel, "github-runner-image-builder"
    )
    assert ret_code == 0, f"Failed to get charm info: {stderr}"
    charmhub_info = json.loads(stdout.strip())
    charmhub_config_options = charmhub_info["charm"]["config"]["Options"].keys()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR introduces support for providing the OpenStack password via Juju secrets, reducing the need to store sensitive credentials in plain-text charm config while keeping legacy configuration supported for backward compatibility.

Changes:

Add a new openstack-password-secret charm config option (type secret) and document deprecation of openstack-password.
Update OpenStack clouds config parsing to prefer fetching the password from a Juju secret when configured.
Update unit and integration tests/factories to exercise secret-based and legacy password paths.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/state.py`	Adds new config constant and updates `_parse_openstack_clouds_config` to fetch password from a Juju secret when provided.
`charmcraft.yaml`	Adds `openstack-password-secret` config option and marks `openstack-password` as deprecated in its description.
`tests/unit/factories.py`	Updates mock charm factory defaults to use the secret-based password path and mocks `model.get_secret()`.
`tests/unit/test_state.py`	Adds unit coverage for secret-fetch error cases, missing password key in secret, legacy password fallback, and missing-password scenarios.
`tests/integration/conftest.py`	Adds integration secret creation/granting for OpenStack password and wires secret-based config into app deployment fixtures.
`docs/changelog.md`	Adds changelog entry describing the new secret-based password configuration and deprecation of the legacy option.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- charmcraft.yaml: remove trailing backslash in YAML literal block for openstack-password deprecation notice to avoid literal backslash in rendered description - docs/changelog.md: add PR link and date to changelog entry for consistency with other entries - src/state.py: remove unnecessary nosec comment on OPENSTACK_PASSWORD_SECRET_CONFIG_NAME (not a password literal); add Juju version check and secret ID format validation before calling model.get_secret() in _parse_openstack_clouds_config so users get clear error messages (upgrade guidance or expected format) - tests/integration/conftest.py: fix dict_keys vs set type annotation in _prepare_charmhub_app_config; add legacy openstack-password fallback when charmhub revision does not expose openstack-password-secret; fix grant order in app_on_charmhub_fixture (deploy without secret config, grant, then set_config to trigger config-changed after permissions are in place); import OPENSTACK_PASSWORD_CONFIG_NAME - tests/unit/factories.py: remove misleading nosec marker from secret ID value (it is a reference, not a credential) - tests/unit/test_state.py: add patch_juju_version_33 to tests using the secret path; add tests for unsupported Juju version and invalid secret ID format Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yanksyoon

Initial review

Agent-Logs-Url: https://github.com/canonical/github-runner-image-builder-operator/sessions/453107d7-866a-4c44-8000-d97b9348860b Co-authored-by: yanksyoon <37652070+yanksyoon@users.noreply.github.com>

Bandit flags 'openstack-password-secret' as B105 (hardcoded_password_string) because the string contains 'password'. Restore the nosec comment on OPENSTACK_PASSWORD_SECRET_CONFIG_NAME in state.py. Also restore the nosec comment in factories.py for the mock secret content dict, which bandit flags as B105 in full-repo scan mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Collapse dict comprehension to single line to satisfy black formatting - Collapse set_config call to single line to satisfy black formatting - Add pylint disable for too-many-arguments and too-many-positional-arguments on app_on_charmhub_fixture which now takes 6 fixture parameters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mypy rejects encoding= when handlers= is also passed to basicConfig, since only the filename-based overload accepts encoding. The encoding is already set on the WatchedFileHandler constructor, so removing it from basicConfig is correct and harmless. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yanksyoon added 3 commits April 17, 2026 12:33

feat: use juju secrets

0a04104

test: lint

c0c4c5c

docs: changelog

ce037f0

github-actions Bot added the Libraries: Out of sync label Apr 17, 2026

yanksyoon marked this pull request as ready for review April 17, 2026 05:00

yanksyoon requested review from cbartz, javierdelapuente and yhaliaw as code owners April 17, 2026 05:00

cbartz requested a review from Copilot April 17, 2026 05:08

Copilot started reviewing on behalf of cbartz April 17, 2026 05:09 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread src/state.py Outdated

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py

Comment thread tests/unit/factories.py Outdated

yanksyoon marked this pull request as draft April 17, 2026 05:15

chore: make config backwards compatible

7faffeb

yanksyoon requested a review from Copilot April 17, 2026 06:03

Copilot started reviewing on behalf of yanksyoon April 17, 2026 06:04 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py Outdated

Comment thread src/state.py Outdated

Comment thread src/state.py

Comment thread charmcraft.yaml Outdated

Comment thread docs/changelog.md Outdated

yanksyoon and others added 2 commits April 17, 2026 06:28

fix: lint

c89d289

yanksyoon commented Apr 20, 2026

View reviewed changes

Comment thread src/state.py Outdated

Comment thread src/state.py

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py

Copilot started work on behalf of yanksyoon April 20, 2026 06:08 View session

yanksyoon commented Apr 20, 2026

View reviewed changes

fix: remove Juju version gate for OpenStack password secrets

c2269ac

Agent-Logs-Url: https://github.com/canonical/github-runner-image-builder-operator/sessions/453107d7-866a-4c44-8000-d97b9348860b Co-authored-by: yanksyoon <37652070+yanksyoon@users.noreply.github.com>

Copilot finished work on behalf of yanksyoon April 20, 2026 06:14

yanksyoon and others added 6 commits April 20, 2026 15:02

test: update secret passing

04a208e

ci: trigger ci

3aa52e2

ci: trigger ci

5cfba75

yanksyoon marked this pull request as ready for review April 20, 2026 18:08

yhaliaw approved these changes Apr 20, 2026

View reviewed changes

yanksyoon merged commit 61a61d6 into main Apr 21, 2026
51 of 59 checks passed

yanksyoon deleted the feat/sensitive-secret-configs branch April 21, 2026 01:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: sensitive secret configs#219

feat: sensitive secret configs#219
yanksyoon merged 13 commits intomainfrom
feat/sensitive-secret-configs

yanksyoon commented Apr 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanksyoon left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

yanksyoon commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Rationale

Juju Events Changes

Module Changes

Library Changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanksyoon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yanksyoon commented Apr 17, 2026 •

edited

Loading